Skip to content

[CALCITE-5212] Include DECIMAL scale in digest when precision unspecified #4937

Open
AlexisCubilla wants to merge 1 commit into
apache:mainfrom
AlexisCubilla:CALCITE-5212-decimal-digest-scale
Open

[CALCITE-5212] Include DECIMAL scale in digest when precision unspecified #4937
AlexisCubilla wants to merge 1 commit into
apache:mainfrom
AlexisCubilla:CALCITE-5212-decimal-digest-scale

Conversation

@AlexisCubilla
Copy link
Copy Markdown

[CALCITE-5212] Include DECIMAL scale in type digest when precision unspecified
Problem:
createSqlType(DECIMAL, PRECISION_NOT_SPECIFIED, scale) could return types that
collided in DATATYPE_CACHE: same digest/hash for different scales because
BasicSqlType.generateTypeString() only appended scale when printPrecision was
true, so scale was omitted when precision was PRECISION_NOT_SPECIFIED.

Root cause:
The digest string did not distinguish DECIMAL types that differed only by
scale while precision stayed unspecified, breaking equals/hashCode/canonize.

Fix:
When precision is not specified but scale is, append "(PRECISION_NOT_SPECIFIED, scale)"
to the digest for DECIMAL (see generateTypeString).

Testing:
./gradlew :core:test --tests "org.apache.calcite.sql.type.SqlTypeFactoryTest"

https://issues.apache.org/jira/browse/CALCITE-5212

…specified

Problem:
createSqlType(DECIMAL, PRECISION_NOT_SPECIFIED, scale) could return types that
collided in DATATYPE_CACHE: same digest/hash for different scales because
BasicSqlType.generateTypeString() only appended scale when printPrecision was
true, so scale was omitted when precision was PRECISION_NOT_SPECIFIED.

Root cause:
The digest string did not distinguish DECIMAL types that differed only by
scale while precision stayed unspecified, breaking equals/hashCode/canonize.

Fix:
When precision is not specified but scale is, append "(PRECISION_NOT_SPECIFIED, scale)"
to the digest for DECIMAL (see generateTypeString).

Testing:
./gradlew :core:test --tests "org.apache.calcite.sql.type.SqlTypeFactoryTest"

https://issues.apache.org/jira/browse/CALCITE-5212
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant